Skip to content

Reuse OpenAI clients to improve throughput - #72

Closed
luojiyin1987 wants to merge 1 commit into
VectifyAI:mainfrom
luojiyin1987:feat/openai-client-reuse
Closed

Reuse OpenAI clients to improve throughput#72
luojiyin1987 wants to merge 1 commit into
VectifyAI:mainfrom
luojiyin1987:feat/openai-client-reuse

Conversation

@luojiyin1987

Copy link
Copy Markdown
Contributor

Summary

  • Add module-level singleton clients cached per API key
  • Replace client creation with _get_sync_client/_get_async_client helper functions
  • Fix chat_history mutation side effect (use list concatenation instead of append)
  • Fix error return type inconsistency in ChatGPT_API_with_finish_reason
  • Add missing re import

Closes #71

- Add module-level singleton clients cached per API key
- Replace client creation with _get_sync_client/_get_async_client
- Fix chat_history mutation side effect (use list concatenation)
- Fix error return type inconsistency in ChatGPT_API_with_finish_reason
- Add missing 're' import
@murthi1832-cmd

Copy link
Copy Markdown

@ncurado

ncurado commented Jan 26, 2026

Copy link
Copy Markdown

Nice improvement on client reuse + chat_history immutability. One small concern: _normalize_api_key returns "" when no key is set, so we end up caching a client under the empty-string key. Suggest guarding in _get_sync_client/_get_async_client (raise ValueError if normalized key is empty) or skip caching in that case, to avoid creating a client with an empty API key.

@KylinMountain

Copy link
Copy Markdown
Collaborator

Thanks @luojiyin1987! Since this was opened, the LLM layer was rewritten to route through LiteLLM (#168), so the functions this patches (ChatGPT_API* with direct openai.OpenAI clients) no longer exist on main and the PR now conflicts.

Each concern is already covered in the current code:

  • Client reuse / throughput (Reuse OpenAI clients to improve throughput #71): LiteLLM reuses its underlying HTTP connections internally, so there's no per-call client creation anymore.
  • chat_history mutation: llm_completion builds messages with list(chat_history) + [...], so the caller's list isn't mutated.
  • Error return consistency: it returns "", "error" on max retries.
  • Missing re import: landed via Adds missing re import #281.

Closing as superseded by the LiteLLM migration — thanks for the solid fixes, they're all reflected in main now. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reuse OpenAI clients to improve throughput

4 participants